Skip to content

fix(dm): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2198

Merged
openai0229 merged 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-dm
Jul 29, 2026
Merged

fix(dm): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2198
openai0229 merged 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-dm

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

Completes the DM database-plugin portion of the identifier and literal escaping work.

  • Centralizes conditional and always-quoted identifier handling in DMIdentifierProcessor, including embedded delimiter escaping and exact quote/remove round trips.
  • Routes metadata, manager, builder, column-type, and index-type SQL generation through the processor instead of direct string interpolation.
  • Adds linear, quote-aware guards for default expressions, comments, type details, index clauses, sort orders, and other SQL fragments that cannot be escaped as identifiers.
  • Preserves inherited Oracle-compatible DML behavior while covering DM-specific metadata and DDL paths.
  • Expands regression coverage for reserved words, mixed-case and embedded-quote identifiers, unknown types, defaults, indexes, and adversarial metadata names.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Commands and results:
    • mvn -pl chat2db-community-plugins/chat2db-community-dm -am clean test -Dmaven.test.skip=false -DskipTests=false
    • Result: BUILD SUCCESS across 9 reactor modules; 184 tests passed in tested modules, including DM 24/24, Oracle 44/44, SPI 90/90, and tools 26/26.
    • git diff --check origin/main...HEAD: passed.
  • Manual verification: Source-level generated-SQL review completed. No live DM instance was used.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No public API or stored-data schema change. Uses the existing shared ISQLIdentifierProcessor contract.
  • Database or driver compatibility: Existing simple identifiers and supported default expressions keep their prior semantics; generated metadata and DDL now quote identifier tokens and validate non-escapable fragments consistently.
  • Network, privacy, or security: No network or privacy change. Reduces malformed SQL and identifier/literal injection risk in generated DM SQL.
  • Community / Local / Pro boundary: Community DM plugin only; Local and Pro consume the Community plugin behavior without product-specific code changes.
  • Backward compatibility: Valid DM identifiers, type definitions, defaults, and index clauses remain supported; malformed or ambiguous raw SQL fragments now fail closed.

Reviewer map

  • Start here: DMIdentifierProcessor, DMSqlGuards, and DMIdentifierProcessorTest, then review call sites in DMMetaData, DMDBManager, and DMSqlBuilder.
  • Failure condition: quote/remove loses raw identifier text, generated SQL exposes metadata outside identifier or literal boundaries, legal DM defaults/types stop rendering, or unvalidated raw fragments reach DDL output.
  • Rollback or disable path: Revert this PR; no migration or persisted-data rollback is required.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: Substantial AI-assisted maintainer revision and test expansion; all final changes were source-reviewed and verified with the commands above.

HandSonic and others added 6 commits July 29, 2026 18:56
… review (OtterMind#1914)

- strengthen DMIdentifierProcessor (SPI ISQLIdentifierProcessor): always-quote
  quoteIdentifier with embedded-quote doubling, escapeString with single-quote
  doubling, static escapeIdentifier for pre-quoted templates, INSTANCE
- DMMetaData.getSQLIdentifierProcessor() returns the shared INSTANCE; metadata
  call sites use it (quoteIdentifier / escapeString / method refs)
- builders/DBManager/enums use DMIdentifierProcessor.INSTANCE / static escapeIdentifier
- non-escapable DEFAULT-expression validation moved to public DMSqlGuards
- DMSqlEscapes removed; tests migrated to DMIdentifierProcessorTest (11 green)
… ReDoS) (OtterMind#1914)

Balanced-paren + quoted-literal aware scanning keeps NVL(SUM(x),0) and
quoted defaults accepted; semicolons and unbalanced input rejected.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased on the latest main and verified the complete DM identifier/literal handling path. The clean 9-module reactor passes, including DM 24/24, Oracle 44/44, SPI 90/90, and tools 26/26.

@openai0229
openai0229 merged commit 22288d9 into OtterMind:main Jul 29, 2026
19 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

3 participants